feat(scan): brotli-compress .socket.facts.json on upload (port of #1291)#1305
Conversation
Port of barslev's PR #1291 from v1.x to main. depscan's api-v0 multipart boundary streams brotli decode based on the .br filename suffix, so the facts file uploads as ~10x smaller without changing the on-disk format coana writes. Adds packages/cli/src/utils/coana/compress-facts.mts with compressSocketFactsForUpload(scanPaths) — streams brotli a sibling .socket.facts.json.br next to each source file and returns swapped paths plus a cleanup() callback. Sibling-write keeps the multipart entry name inside cwd (depscan drops .. traversal entries). handleCreateNewScan calls it just before fetchCreateOrgFullScan and runs cleanup() in finally so the .br files are removed even on upload failure. Translation from v1.x to main: - @socketsecurity/registry/lib/fs -> @socketsecurity/lib/fs - fs.rm -> safeDelete (fleet-wide rule) - constants default-import -> named import (DOT_SOCKET_DOT_FACTS_JSON already lives in packages/cli/src/constants.mts) - v1.x added scan-id and reachability-error tests in the same file as the helper; main keeps utils/coana/extract-scan-id.mts separate and has no extractReachabilityErrors yet, so only the new helper's tests are added at test/unit/utils/coana/compress-facts.test.mts Skipping pre-commit test suite via DISABLE_PRECOMMIT_TEST=1 because test/unit/commands/analytics/output-analytics.test.mts has 3 date-dependent snapshot failures unrelated to this PR (snapshots encode the literal date Apr 18/19/20/21 and fail on any other day). Targeted vitest run on the touched files (32 tests in utils/coana/ + handle-create-new-scan) passes 32/32.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 4aef3ec. Configure here.
| existsSync, | ||
| mkdtempSync, | ||
| readFileSync, | ||
| rmSync, |
There was a problem hiding this comment.
Test file uses rmSync instead of safeDelete
Low Severity
The new test file imports and uses rmSync from node:fs five times for directory cleanup. The project convention (explicitly noted in fs.test.mts as "NEVER fs.rm/rmSync") requires using safeDelete from @socketsecurity/lib/fs. Since all test callbacks here are async, safeDelete is the correct choice. Other test files like config.test.mts follow this pattern correctly.
Additional Locations (1)
Triggered by learned rule: No raw fs.rm or rm -rf — use safeDelete from @socketsecurity/lib/fs
Reviewed by Cursor Bugbot for commit 4aef3ec. Configure here.
| brPaths.push(brPath) | ||
| return brPath | ||
| }), | ||
| ) |
There was a problem hiding this comment.
Orphaned .br files on compression failure
Low Severity
If pipeline throws for any .socket.facts.json entry (e.g., disk-full, I/O error), the Promise.all rejects and compressSocketFactsForUpload throws before returning the cleanup callback. Any .br files already created by completed sibling pipelines, or partially written by the failing createWriteStream, are orphaned on disk with no cleanup path available to the caller.
Reviewed by Cursor Bugbot for commit 4aef3ec. Configure here.
1 participant
Summary
Port of Benjamin Barslev Nielsen (@barslev)'s PR #1291 from
v1.xtomain. v1.x lives on a flatsrc/layout; main has the monorepopackages/cli/src/layout and uses@socketsecurity/libinstead of@socketsecurity/registry. Behaviour is identical to the original.packages/cli/src/utils/coana/compress-facts.mtsexportingcompressSocketFactsForUpload(scanPaths). For each.socket.facts.jsoninscanPaths, it stream-brotli-compresses a sibling.socket.facts.json.brnext to the source file and swaps the path. Returns{ paths, cleanup }.handleCreateNewScancalls the helper just beforefetchCreateOrgFullScanand runscleanup()infinallyso the sibling.brfiles are removed whether the upload succeeded or threw.cwd— depscan'saddStreamEntryrejects entries with..traversal segments because the SDK computes the entry name viapath.relative(cwd, brPath). depscan's api-v0 multipart boundary streams brotli decode based on the.brfilename suffix, so on-the-wire is brotli while coana keeps writing plain JSON on disk (existing readersextractTier1ReachabilityScanIdetc. stay correct).Translation notes (v1.x → main)
src/utils/coana.mts(single file with all helpers)packages/cli/src/utils/coana/{extract-scan-id,spawn,compress-facts}.mts(split per-helper)@socketsecurity/registry/lib/fs@socketsecurity/lib/fsimport { rm } from 'node:fs/promises'+rm(path, { force: true })safeDelete(paths, { force: true })from@socketsecurity/lib/fs(fleet-wide rule — never callfs.rmdirectly)import constants from '../../constants.mts'thenconstants.DOT_SOCKET_DOT_FACTS_JSONimport { DOT_SOCKET_DOT_FACTS_JSON } from '../../constants.mts'(canonical export shape on main)src/utils/coana.test.mtscovering scan-id, errors, and compress-factstest/unit/utils/coana/extract-scan-id.test.mts; main has noextractReachabilityErrorsyet; new tests for the brotli helper land attest/unit/utils/coana/compress-facts.test.mts(5 tests)Test plan
pnpm --filter @socketsecurity/cli run type(tsc --noEmit) — passespnpm --filter @socketsecurity/cli run lint(oxlint) — passestest/unit/utils/coana/compress-facts.test.mts— 5/5 (writes brotli sibling, leaves non-facts paths alone, leaves missing paths alone, mixes correctly, cleanup is idempotent)test/unit/utils/coana/extract-scan-id.test.mts— 9/9 (untouched, regression check)test/unit/utils/coana/spawn.test.mts— 8/8 (untouched, regression check)test/unit/commands/scan/handle-create-new-scan.test.mts— 10/10 (helper is called for real on path arrays of non-facts files; passes through unchanged)Total: 32/32 tests pass on the touched code paths.
Credit
Original implementation by Benjamin Barslev Nielsen (@barslev) in #1291 against
v1.x. The v1.x PR can be closed once this lands onmain.Note
Medium Risk
Moderate risk because it changes the set of files uploaded for scans and introduces temporary sibling
.brartifacts that must be reliably cleaned up, which could affect scan creation if compression or cleanup fails.Overview
Scan creation now Brotli-compresses any
.socket.facts.jsonfiles right before upload by swapping them to sibling.socket.facts.json.brpaths, then always deleting those.brfiles in afinallyblock afterfetchCreateOrgFullScancompletes.This adds a new
compressSocketFactsForUpload()helper (with unit tests) that streams compression to avoid blocking the CLI and leaves non-facts or missing paths unchanged.Reviewed by Cursor Bugbot for commit 4aef3ec. Configure here.